Repository navigation
Fix output-axis reductions and DR variance propagation - #1069
kbattocchi wants to merge 4 commits into
Conversation
366cd5b to
4ca2540
Compare
4ca2540 to
c37542f
Compare
55ec498 to
464bd8f
Compare
464bd8f to
46bb96a
Compare
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
2 open issues
What changed in this PR
Fixes several axis/broadcasting bugs affecting model-selection scoring, population mixture confidence intervals, and DRLearner variance propagation—plus adds regression tests for the corrected behaviors.
Changes:
- Normalize ElasticNetCV/LassoCV selector “R²” scoring by mean per-output variance (instead of pooled variance).
- Make Gaussian-mixture CI computation invalidate only affected outcome/treatment columns and avoid nontermination on invalid normal params.
- Correct DRLearner final-stage
sample_varshape handling and propagate outcome variance per treatment-vs-control contrast.
| File | Description |
|---|---|
| econml/sklearn_extensions/model_selection.py | Fixes selector score normalization for multi-output targets. |
| econml/inference/_inference.py | Refines mixture PPF invalidation to column-level and updates docstring. |
| econml/dr/_drlearner.py | Fixes DR sample_var shape validation and per-contrast variance propagation. |
| econml/tests/test_model_selection.py | Adds regression tests for selector scoring invariance to output offsets/reordering. |
| econml/tests/test_inference.py | Adds tests for mixture CI behavior with zero/invalid scales per column. |
| econml/tests/test_drlearner.py | Adds tests for DR final-stage sample_var shapes and variance equivalence. |
Review effort: Lite
| # Invalid normal parameters prevent convergence. Exclude only the affected | ||
| # outcome/treatment columns from the solver. | ||
| invalid = np.any((stderr <= 0) | ~np.isfinite(stderr) | ~np.isfinite(mean), axis=0) | ||
| if np.any(invalid): | ||
| result = np.full(shape(mean)[1:], np.nan) | ||
| if not np.all(invalid): | ||
| result[~invalid] = self._mixture_ppf(alpha, mean[:, ~invalid], stderr[:, ~invalid], tol) | ||
| return result |
There was a problem hiding this comment.
This indexing is valid: NumPy's boolean masks consume as many consecutive axes as the mask has dimensions. Here invalid.shape == mean.shape[1:], so mean[:, ~invalid] preserves the sample axis and selects valid outcome/treatment pairs, returning (m, n_valid). result[~invalid] maps the results back to the original output shape. Explicit flattening would be equivalent, not a fix.
I verified the selection/assignment directly and executed all 34 existing mixture regression cases against this PR's helper, including multidimensional outputs and reordered invalid columns; they pass. See NumPy's boolean-indexing rules. Keeping the implementation unchanged.
| Get the confidence interval of the point estimate of each treatment on each outcome for sample X. | ||
|
|
||
| An outcome/treatment interval is NaN if any of its sample means or standard errors | ||
| is nonfinite, or any standard error is nonpositive. Other intervals are unaffected. |
There was a problem hiding this comment.
Clarified in d517e90: “An outcome/treatment interval is NaN if any sample mean or standard error is nonfinite, or any standard error is nonpositive. Other intervals are unaffected.” This avoids the ambiguous agreement without changing behavior.
d517e90 to
9602a0b
Compare
Normalize output-averaged CV MSE by mean within-output variance so independent outcome offsets do not inflate selector scores. Preserve the fast approximation and existing unweighted normalization policy. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Keith Battocchi <kebatt@microsoft.com>
Keep NaN intervals local to outcome/treatment columns with nonpositive scales or nonfinite normal parameters. Exclude invalid columns before solving so unaffected intervals retain their values and shape without risking nontermination. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Keith Battocchi <kebatt@microsoft.com>
Align vector and singleton-column sample variances with final targets, and apply the squared observed-outcome derivative separately for each treatment-versus-control contrast. This avoids quadratic broadcasting and excludes variance from unrelated treatment arms. Cover both final-model modes, trimming, weighted summaries, and expanded-observation covariance equivalence. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Keith Battocchi <kebatt@microsoft.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Keith Battocchi <kebatt@microsoft.com>
9602a0b to
cc33889
Compare
Summary
Following up on #1067, had Copilot audit the codebase for other places where we wrongly use a numpy operation on a whole array instead of specifying an
axis.This PR fixes the four confirmed bugs that the review found, in three logical commits:
sample_varvectors and singleton columns with final targets, avoiding unintended sample-by-sample broadcasting and fitting failures.Includes regression tests and directly related documentation. Both separate and multitask DR final models are covered, including trimming, heterogeneous weights/propensities, and summarized-versus-expanded observation equivalence.